Skip to content

dbeaver/pro#7562 [CB] keep data editor filter state during session - #4261

Merged
devnaumov merged 45 commits into
develfrom
7562-cb-keep-data-editor-filter-state-during-session
Apr 22, 2026
Merged

dbeaver/pro#7562 [CB] keep data editor filter state during session#4261
devnaumov merged 45 commits into
develfrom
7562-cb-keep-data-editor-filter-state-during-session

Conversation

@SychevAndrey

Copy link
Copy Markdown
Contributor

No description provided.

SychevAndrey and others added 25 commits March 24, 2026 17:33
replaced private restorePendingState()  with public restoreViewState(pinnedColumnNames, columnOrderNames?) that takes data directly as arguments
bug with skipping constraints if any without a position.
if (!prevColumn) {
        return;
      }
…of github.com:dbeaver/cloudbeaver into 7562-cb-keep-data-editor-filter-state-after-reconnect
…LDataFilter"

This reverts commit 3e0e86f.

Last test showed that we don't need any additional logic on BE. It handles all constraints properly.
@codacy-production

codacy-production Bot commented Apr 3, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric Results
Complexity 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes. Give us feedback

@SychevAndrey
SychevAndrey requested review from Wroud and devnaumov April 10, 2026 12:10
…DatabasePersistedStateStore and update related references
@SychevAndrey
SychevAndrey requested a review from Wroud April 14, 2026 07:41
SychevAndrey and others added 2 commits April 14, 2026 09:41
…f github.com:dbeaver/cloudbeaver into 7562-cb-keep-data-editor-filter-state-during-session
Comment on lines +47 to +64
private applyPersistedConstraints(): void {
const options = this.source.options;

if (!options || !validatePersistedState(this.store)) {
return;
}

if ('constraints' in options && 'whereFilter' in options) {
options.constraints = this.store.constraints.map(c => ({
attributeName: c.attributeName,
operator: c.operator,
value: c.value,
orderAsc: c.orderAsc,
orderPosition: c.orderPosition,
}));
options.whereFilter = this.store.whereFilter || '';
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move to result set data source

Comment on lines +344 to +351
for (const name of pinnedColumnNames) {
const key = this.columnKeys.find(k => this.getColumnName(k) === name);

if (key) {
this.pinnedColumns.add(GridDataKeysUtils.serialize(key));
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will suggest to move validation logic for pinned columns and columns order to the updateResult method

ps.set(COLUMN_ORDER_KEY, isCustomOrder ? columnNames : []);
}

restoreViewState(state: IRestoreViewState): void {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to use direct states from store instead of syncing private properties with state

return this.result.data?.columns?.find(c => c.position === colIdx)?.name;
}

private persistConstraints(): void {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to persist datasource options object (whereFilter) maybe it will be better to persist constraints in the same place instead of splitting it across classes

- DatabasePersistedStateStore: pure KV (observable.ref + actions)
- DatabaseDataSource: add loadPersistedState() + onPersistedStateLoaded() hook
- DatabaseDataConstraintAction: own the constraint persist
- ResultSetDataSource: thin dispatcher
- GridViewAction: columnsOrder/pinnedColumns become computed getters
…ards, simplify persisted data filter state handling
@SychevAndrey
SychevAndrey requested a review from Wroud April 17, 2026 08:34
Wroud
Wroud previously approved these changes Apr 20, 2026
executionContext: observable,
});

this.persistConstraintsDisposer = autorun(() => persistDataFilterConstraints(this));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will recommend to initialize it after applyPersistedDataFilterConstraints(this); to avoid collisions

but honestly this is bad that we use DatabaseDataConstraintAction in the DataSource (it was so before you changes, but now there is more dependencies on it)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The separate ticket will be created for this and bellow comments

Comment on lines +42 to +57
const options = source.options;
if (!options) {
return;
}

const constraints = source.persistedState.get<SqlDataFilterConstraint[]>(CONSTRAINTS_KEY);
const whereFilter = source.persistedState.get<string>(WHERE_FILTER_KEY);

if (!Array.isArray(constraints) || typeof whereFilter !== 'string') {
return;
}

runInAction(() => {
options.constraints = constraints.map(constraint => ({ ...constraint }));
options.whereFilter = whereFilter;
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about:

Suggested change
const options = source.options;
if (!options) {
return;
}
const constraints = source.persistedState.get<SqlDataFilterConstraint[]>(CONSTRAINTS_KEY);
const whereFilter = source.persistedState.get<string>(WHERE_FILTER_KEY);
if (!Array.isArray(constraints) || typeof whereFilter !== 'string') {
return;
}
runInAction(() => {
options.constraints = constraints.map(constraint => ({ ...constraint }));
options.whereFilter = whereFilter;
});
const constraints = source.persistedState.get<SqlDataFilterConstraint[]>(CONSTRAINTS_KEY);
const whereFilter = source.persistedState.get<string>(WHERE_FILTER_KEY);
if (!Array.isArray(constraints) || typeof whereFilter !== 'string') {
return;
}
source.setOptions({
...source.options,
constraints: constraints.map(constraint => ({ ...constraint })),
whereFilter
});

Comment on lines +360 to +361
source.options!.constraints = [];
source.options!.whereFilter = '';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will recommend to use .setOptions

Comment on lines +243 to +244
loadPersistedState(state: Record<string, unknown>): this {
this.persistedState.setStore(state);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setStore but state: Record<string, unknown>

devnaumov
devnaumov previously approved these changes Apr 20, 2026
@devnaumov
devnaumov dismissed stale reviews from Wroud and themself via 9bfc3c7 April 20, 2026 16:48
@devnaumov
devnaumov merged commit 9c6f301 into devel Apr 22, 2026
9 of 10 checks passed
@devnaumov
devnaumov deleted the 7562-cb-keep-data-editor-filter-state-during-session branch April 22, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants